Draft: [Event Highlighting]: Add a web worker for outsourcing the marker calculation #223

thgrund · 2025-08-08T19:47:45Z

I optimized the even highlighting performance with replacing the message queue mechanism. The calculation will be done completely by a web worker, that stores all incoming events and calculates the sets for added, active and removed events. These sets will be used for adding and removing markers.

This increases the user experience especially during editing big files.

…feature is enabled.

Add styles Use MrReason setup directory as stack enrtry point Change stack package path Continue rebasing Continue rebasing Continue rebasing Continue rebasing Continue rebasing Continue rebasing Continue rebasing Continue rebasing Continue rebasing Continue rebasing Continue rebasing Continue rebasing Continue rebasing Continue rebasing Fix remaining rebase issues Fix remote control Add safety check Handle osc style changes Add a event highlighting web worker Replace old event queue with worker queue Add set tests Remove rebase duplication

Remove OSC

thgrund · 2025-08-08T19:49:26Z

@ndr-brt I really appreciate your review and feedback!
I splitted my TODOs so that the MRs will be easier for reviewing. My suggestion is to wait for a release, until the optimizations and new featuers are implemented. WDYT?

And sorry for the mess with the git history. I cleaned everything with interactive rebase...and then messed it up in the last step xD - I would really appreaciate, if you could do a squash merge when merging.

ndr-brt

@thgrund ok for me to wait, I'll only release when you'll fill comfortable with the progress.

np with the git history: I always squash in a single commit :)

ndr-brt · 2025-08-09T22:31:05Z

lib/event-highlighting/event-highlight-worker.js

+let messageBuffer = new Map();
+let receivedThisFrame = new Map();
+
+self.onmessage = function(e) {


I'm not really an expert in js workers, but I see that after the instantiation (event-highlighter.js:52), this onmessage field is overridden. Is then this assignment useless?

Nope, this is the function that will be triggered, when the web worker receives messages from the main thread. The other one in in event-highlighter.js:52 is the function that will be triggered, when the main thread receives a message from the web worker.

This is the hole main idea. Doing the calculation and message buffer handling within a separate thread and independant from the animation frame.

ndr-brt · 2025-08-09T22:31:36Z

lib/event-highlighting/event-highlight-worker.js

+
+/** Store events until the next animation frame */
+function queueEvent(event) {
+  const messageBufferMap = ensureNestedMap(messageBuffer, event.eventId);


this const is never used

ndr-brt · 2025-08-09T22:31:59Z

lib/event-highlighting/event-highlight-worker.js

+    messageBuffer = EventMapHelper.transformedEvents(added.union(active));
+    receivedThisFrame.clear();
+
+}, 1000/30);


magic number, could it be extracted in a const with a meaningful name?

I added a const for this. I wasn't sure how I wanna handle this. Now the animation is locked by a maximum of 30FPS. Maybe I will make it configurable at a later point again, but the web worker has no access of the atom config properties, because they are only available within the main thread.

ndr-brt · 2025-08-09T22:33:04Z

lib/event-highlighting/event-highlighter.js

+    this.startWorker();
+  }
+
+  startWorker() {


this method can be private

ndr-brt · 2025-08-09T22:39:03Z

lib/event-highlighting/event-highlighter.js

-    this.filteredMessages = new Map();     // eventId → event
-    this.receivedThisFrame = new Map();    // eventId → event
-    this.addedThisFrame = new Set();       // Set<event>
+    this.oscStyleIds = new Set();          // received ids


this field is never used

Removed it.

Yeah...For the sake of transparency, I implement features within a separate feature branch to try out some certain implementations. And I messed up the previous rebase, this is why this stuff unfortunately appears within this MR

ndr-brt · 2025-08-09T22:44:52Z

lib/event-highlighting/event-map-helper.js

@@ -0,0 +1,59 @@
+'use babel'
+
+class EventMapHelper {


I admit, I'm not the biggest fan of "helpers" and "utils" classes, so I'd suggest to avoid them as much as possible.

In this case, both the functions are only used by the worker, so could them be moved there?

Me neither. But I am not able to define functions within the web worker and test them properly, because the jasmine tests only works within the main thread. This means I can not simply export them, because a worker is not a module. This would be the tradeoff: extract them somehow (class method or static function) or removing the tests. And to test the worker within the main thread is already an integration test.

ndr-brt · 2025-08-09T22:46:03Z

lib/event-highlighting/event-map-helper.js

+      return { removed, added, active};
+  }
+
+  static transformedEvents(events) {


the name is quite vague, could it be called setToMap or groupByEventIdAndColStart (maybe the first is better :) )

ndr-brt · 2025-08-09T22:50:15Z

lib/polyfills/set.js

@@ -0,0 +1,15 @@
+if (!Set.prototype.union) {


of these 3 methods, only union is actually used, and just once.
tbh I would avoid the hassle to define and have to require the polyfills set here and there and just inline the union method where it's used (worker)

For me this felt odd to not have difference and intersections, but I agree. I removed the other ones.
Unfortunately when I move this union definition into the worker file, then it is not possible to add a unit test for this (see: #223 (comment))

ndr-brt · 2025-08-09T22:52:00Z

lib/osc-eval.js

        }

-        tidalRepl.eval(message['type'], false);
+        if (message['search']) {


is this changelog related to the scope of the PR?

Nope, I removed it.

thgrund and others added 18 commits August 8, 2025 19:57

Add event highlighting to the TidalCycles package

1c392c5

Delete styles/tidal.less

a501056

Restore tidal.less file

e6cd4b3

Add condition to ensure, that meta data will only be added, when the …

9a0c4a2

…feature is enabled.

Refactor osc handling

f2a275a

Introduce eventIds

2d7c2c3

Add multiple text editor support

43d68f2

Fix PR issues

96ef2a4

Refactor osc server transformer logic

7739556

chore: move highlight feature enable check to main

bfe6785

Add LineProcessor helper class

4006795

Extract regex statements

ee0c4f0

Add osc style description to README

7023d6a

Remove OSC

Remove remote control styles

6d71198

Remove files from MrReason Setup

ee5cf41

Revert packages from MrReason Setup

82813bc

Clean up set polyfill

e8a03ff

ndr-brt self-requested a review August 9, 2025 13:15

ndr-brt reviewed Aug 9, 2025

View reviewed changes

Fix MR review

36159a6

thgrund changed the title ~~Optimize event highlighting performance~~ [Event Highlighting]: Optimize event highlighting performance Aug 11, 2025

thgrund changed the title ~~[Event Highlighting]: Optimize event highlighting performance~~ [Event Highlighting]: Optimize performance Aug 11, 2025

thgrund changed the title ~~[Event Highlighting]: Optimize performance~~ Draft: [Event Highlighting]: Optimize performance Aug 12, 2025

thgrund marked this pull request as draft August 12, 2025 14:27

thgrund changed the title ~~Draft: [Event Highlighting]: Optimize performance~~ Draft: [Event Highlighting]: Add a web worker for outsourcing the marker calculation Aug 13, 2025

thgrund mentioned this pull request Aug 14, 2025

Event Highlighting: Optimize performance #229

Open

Draft: [Event Highlighting]: Add a web worker for outsourcing the marker calculation #223

Are you sure you want to change the base?

Draft: [Event Highlighting]: Add a web worker for outsourcing the marker calculation #223

Uh oh!

Conversation

thgrund commented Aug 8, 2025

Uh oh!

thgrund commented Aug 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ndr-brt left a comment

Choose a reason for hiding this comment

Uh oh!

ndr-brt Aug 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thgrund Aug 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

thgrund commented Aug 8, 2025 •

edited

Loading

ndr-brt Aug 9, 2025 •

edited

Loading

thgrund Aug 10, 2025 •

edited

Loading